-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/wishbone #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/wishbone #38
Conversation
79ae7aa to
7f87dd9
Compare
Paebbels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the proposed names aren't matching the specification for 100% anymore, I suggest to prefer a clean naming scheme. This isn't chaning the name/meaning itself, just changing abbreviations to full non-abbreviated names.
compileorder.list
Outdated
| Wishbone/vB3/Wishbone_Generic.vhdl | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Wishbone/vB3/Wishbone_Generic.vhdl | |
| Wishbone/vB3/Wishbone_Generic.vhdl |
Co-authored-by: Patrick Lehmann <[email protected]>
|
Why are names that differ from the Wishbone spec being used? I too do not like shortened names, however, I also cannot see using any names other than the standard names. I see there being some freedom when the interface is a split transaction interface (like AXI) and multiple layers of records are used. |
We decided to adopt full UpperCamelCase names (Cycle, Strobe, Acknowledge, WriteEnable, etc.) instead of the abbreviated spec names. The spec signal names are still documented in comments for reference, so the mapping should be clear to anyone familiar with Wishbone. |
Paebbels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I solved the conflict for this branch after Avalon was merged.
| WriteEnable : std_ulogic; -- WE_O - Write Enable | ||
| Address : Address_Type; -- ADR_O - Address | ||
| DataOut : Data_Type; -- DAT_O - Data (Master to Slave) | ||
| Select : Select_Type; -- SEL_O - Select |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it fail for signal Select, because it's a reserved word?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

Interface now uses std_ulogic/std_ulogic_vector, addresses are now unsigned and UpperCamelCase is used to increase readability. Contributions on feature/wishbone are now splitted into a new PR.